Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Switch global_ocean and hurricane to framework base and culled mesh #429

Merged
merged 8 commits into from
Aug 30, 2022
Merged

Switch global_ocean and hurricane to framework base and culled mesh #429

merged 8 commits into from
Aug 30, 2022

Conversation

xylar
Copy link
Collaborator

@xylar xylar commented Aug 3, 2022

This allows us to use the more generalized spherical mesh step class with better JIGSAW support from #422 and breaks the mesh test into base_mesh and cull_mesh steps.

@xylar xylar added ocean in progress This PR is not ready for review or merging labels Aug 3, 2022
@xylar xylar requested a review from sbrus89 August 3, 2022 06:08
@xylar xylar self-assigned this Aug 3, 2022
@pep8speaks
Copy link

pep8speaks commented Aug 3, 2022 •
edited
Loading

Hello @xylar! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2022-08-17 09:00:03 UTC

@xylar
Copy link
Collaborator Author

xylar commented Aug 3, 2022

This still needs documentation updates.

@xylar xylar force-pushed the global_ocean_mesh_reorg branch from 0fe4e66 to a586ec1 Compare August 3, 2022 18:48
@xylar xylar marked this pull request as ready for review August 14, 2022 11:04
@xylar xylar removed the in progress This PR is not ready for review or merging label Aug 14, 2022
@xylar
Copy link
Collaborator Author

xylar commented Aug 14, 2022 •
edited
Loading

Testing

The ocean pr test suite passes on Anvil with Intel and Intel-MPI. It is bit-for-bit except for the mesh test cases (because of the reorganization and the new cache files).

I also ran all 3 hurricane test cases (mesh, init and sandy). I tried using a baseline from master but there are no validation methods defined so no comparison is made to the baseline and I have no way of knowing if the results are bit-for-bit or not.

@xylar xylar force-pushed the global_ocean_mesh_reorg branch 2 times, most recently from 1405284 to 02eaf6f Compare August 15, 2022 13:57
@xylar
Copy link
Collaborator Author

xylar commented Aug 15, 2022

@sbrus89, is this something you have any bandwidth to review?

@sbrus89
Copy link
Collaborator

sbrus89 commented Aug 16, 2022

@xylar, yes. I’ll take a look.

@xylar
Copy link
Collaborator Author

xylar commented Aug 16, 2022

I will rebase and deal with conflicts tomorrow.

@xylar xylar force-pushed the global_ocean_mesh_reorg branch from 02eaf6f to 38c34d2 Compare August 17, 2022 07:42
@xylar xylar force-pushed the global_ocean_mesh_reorg branch from bd63c3a to db62191 Compare August 17, 2022 08:59
@xylar
Copy link
Collaborator Author

xylar commented Aug 17, 2022

@sbrus89, I rebased this branch after #413 and (after a few missteps) all the tests pass as before (see #429 (comment) above).

@xylar xylar mentioned this pull request Aug 17, 2022
Merged
sbrus89
sbrus89 approved these changes Aug 30, 2022
View reviewed changes
Copy link
Collaborator

@sbrus89 sbrus89 left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@xylar, I have tested this and approve the qualitative differences in the hurricane case solution resulting from a non BFB mesh. Thanks for making this change!

@xylar
Copy link
Collaborator Author

xylar commented Aug 30, 2022

Thanks @sbrus89! I'll go ahead and merge this.

@xylar xylar merged commit c3dceb8 into MPAS-Dev:master Aug 30, 2022
@xylar xylar deleted the global_ocean_mesh_reorg branch August 30, 2022 15:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sbrus89 sbrus89 sbrus89 approved these changes

Assignees

@xylar xylar

Labels
ocean
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@xylar @pep8speaks @sbrus89